Skip to content

fix(srt): align B200 and MI355X recipe images with master configs / fix(srt):对齐 B200 与 MI355X 配方镜像与主配置 - #3567

Open
chunfangamd wants to merge 9 commits into
mainfrom
chun/recipe-image-consistency
Open

chunfangamd wants to merge 9 commits into
mainfrom
chun/recipe-image-consistency

Conversation

@chunfangamd

@chunfangamd chunfangamd commented Sep 29, 2026 •

Copy link
Copy Markdown
Collaborator

Description

Two single-node AgentX srt-slurm recipes name a container that differs from their master-config image. infx/srt_slurm/single_node.py therefore rejects every point of these keys before Slurm submission (Single-node SRT image: recipe/matrix ...), the same failure #3446 hit in its first sweep.

Cause. #3428 ported these recipes from the legacy scripts as they were before the image bumps in #3420 and #3334, which merged about ten hours earlier. Those bumps changed only the master images, which was correct while the keys still ran the legacy scripts. The PRs touched different files, so git saw no conflict, and no check compares the two copies before a GPU job starts.

Config key Master image (unchanged) Recipe model.container on main
dsv41flash-fp4-mi355x-vllm-agentic-dspark vllm/vllm-openai-rocm:nightly-rocm100-29468dde… vllm/vllm-openai-rocm:nightly-rocm100-7f1a5398…
dsv4-fp4-b200-sglang-agentic-hicache-mtp lmsysorg/sglang:v0.5.20-cu130 lmsysorg/sglang:v0.5.19-cu130

Changes

  1. Recipes: set model.container to the master image. The B200 SGLang v0.5.20 recipe also renames cuda-graph-max-bs to cuda-graph-max-bs-decode in all 12 variants, as [Klaud Cold] Update dsv4-fp4-b200-sglang-agentic-hicache-mtp SGLang image to v0.5.20-cu130 / 将 dsv4-fp4-b200-sglang-agentic-hicache-mtp 的 SGLang 镜像更新至 v0.5.20-cu130 #3334 did in the legacy script. SGLang v0.5.20 no longer accepts the deprecated alias ([Config] Retire get_global_server_args, and clear the deprecated flags that have a replacement sgl-project/sglang#38375), so an image-only fix would fail at server startup. No other serving flag or sweep point changes.
  2. perf-changelog.yaml: one entry for the two keys so the sweep re-validates them on the native srt-slurm path.

Earlier commits also added a static recipe/master image test. Review showed that a pytest scan cannot gate run-sweep, compares image sets per recipe instead of each matrix point's selected variant, misses EVAL_CONFIG_FILE, and pins checked-in config against the AGENTS.md test rules. It is removed here and replaced by #3624, a validator that run-sweep calls before dispatch.

Validation

Notes for reviewers

AI model disclosure

  • Model/version: Claude Opus 5.5 (the identifier exposed by the Cursor agent runtime). No delegated agents were used.
  • Role: investigated the recipe/master image drift, wrote the recipe fixes and the changelog entry, ran the local validation, diagnosed the sweep failures, and drafted this description. @chunfangamd reviewed the changes and chose the scope of this PR.

Related Issue

No issue. Follow-up: #3624. Related: #3428, #3446, #3555.

Type of Change

  • Bug fix
  • New feature
  • Configuration change
  • Documentation update
  • Other (please describe)

Checklist

  • I have completed the AI model disclosure and kept it current
  • I have tested my changes locally
  • I have updated documentation if necessary
  • For every change that can affect benchmark performance and every recipe addition or modification, I have appended a new entry to the physical end of inferencex-e2e/perf-changelog.yaml and have not edited historical entries
  • Before merging via reuse, an authorized maintainer (OWNER/MEMBER/COLLABORATOR) has commented /use <run_id> (or the legacy /reuse-sweep-run) on this PR. Do this only once there is a final full sweep that is all green with evals passing, since after this comment the sweep label will no longer automatically kick off new sweeps. Remove and re-add the label to force one.
中文

改动说明

两个单节点 AgentX srt-slurm 配方的 container 与主配置 image 不一致,导致 infx/srt_slurm/single_node.py 在提交 Slurm 之前拒绝这些 key 的每一个点(Single-node SRT image: recipe/matrix ...),与 #3446 第一次 sweep 遇到的失败相同。

原因: #3428 移植这些配方时,依据的是 #3420 和 #3334 升级镜像之前的旧脚本,而这两个升级约在 #3428 合入前十小时已经合入。升级 PR 只改了主配置镜像,这在这些 key 仍运行旧脚本时是正确的。两边改的是不同文件,git 没有冲突,而在 GPU 任务开始之前也没有任何检查比较两份拷贝。上表列出了两个 key 的主配置镜像(未改动)和 main 上配方的旧镜像。

改动:

  1. 配方: 将 model.container 改为主配置镜像。B200 的 SGLang v0.5.20 配方同时在全部 12 个 variant 中将 cuda-graph-max-bs 改为 cuda-graph-max-bs-decode,与 [Klaud Cold] Update dsv4-fp4-b200-sglang-agentic-hicache-mtp SGLang image to v0.5.20-cu130 / 将 dsv4-fp4-b200-sglang-agentic-hicache-mtp 的 SGLang 镜像更新至 v0.5.20-cu130 #3334 对旧脚本的修改一致。SGLang v0.5.20 已移除该弃用别名([Config] Retire get_global_server_args, and clear the deprecated flags that have a replacement sgl-project/sglang#38375),只改镜像会导致服务启动失败。其余服务参数和 sweep 点均不变。
  2. perf-changelog.yaml: 为这两个 key 追加一条记录,让 sweep 在新的 srt-slurm 路径上重新验证。

之前的 commit 还加了一个配方与主配置镜像的静态测试。Review 指出:pytest 扫描无法拦住 run-sweep;它按配方比较镜像集合,而不是每个 matrix 点实际选中的 variant;没有检查 EVAL_CONFIG_FILE;并且在测试里固定了 checked-in 配置,违反 AGENTS.md 的测试规则。因此本 PR 移除该测试,改由 #3624 提供一个在 run-sweep 派发前调用的 validator。

验证:

审阅注意事项:

AI 模型使用说明

  • 模型/版本:Claude Opus 5.5(Cursor agent 运行环境提供的标识),未使用其他委派 agent。
  • 工作内容:排查配方与主配置镜像不一致,编写配方修复和 changelog 记录,运行本地验证,诊断 sweep 失败,并起草本说明;@chunfangamd 审阅了全部改动并确定了本 PR 的范围。

关联 issue

无。后续 PR:#3624。相关 PR:#3428、#3446、#3555。

改动类型

Bug 修复、配置修改。

chunfangamd and others added 3 commits September 29, 2026 01:26
#3428 ported these AgentX recipes from the legacy scripts as they were before the 09-25 image bumps (#3361, #3362, #3334, #3420), which changed only the master images. Every point of these keys now fails before submission with 'Single-node SRT image: recipe/matrix'.

The SGLang v0.5.20 recipes also take the --cuda-graph-max-bs-decode rename that #3362 and #3334 applied to the legacy scripts; v0.5.20 no longer accepts the deprecated --cuda-graph-max-bs alias (sgl-project/sglang#38375).

Co-authored-by: Cursor <[email protected]>
single_node.py rejects a point whose recipe container differs from the matrix image only once a GPU job starts, and the multi-node path has no such check: srtctl pulls a literal container missing from the alias map. Check both statically, including identity.container.image.

Co-authored-by: Cursor <[email protected]>
@github-actions

Copy link
Copy Markdown
Contributor

Thanks for the contribution!

  • Review: If this PR changes files owned by someone other than a repository admin or @SemiAnalysisAI/core, ask one eligible CODEOWNER to complete the latest PR_REVIEW_CHECKLIST.md before contacting a core maintainer on Slack. Follow the template exactly, including As a PR reviewer and CODEOWNER, I have reviewed this and have, so sign-off verification triggers.
  • PR verification: Sweeps only run on labeled PRs. Add full-sweep-fail-fast (strongly recommended); use full-sweep-enabled only when matrix jobs should continue after a failure.
  • After merging: PR authors must ensure all GitHub Actions jobs pass. Transient failures often pass on rerun; see how to rerun failed jobs.
中文

感谢你的贡献!

  • **审阅:**如果 PR 修改的文件归属于仓库管理员及 @SemiAnalysisAI/core 之外的 CODEOWNER,请先联系一位有资格的 CODEOWNER 填写最新的 PR_REVIEW_CHECKLIST.md,再通过 Slack 联系核心维护者。必须严格遵循模板,并保留 As a PR reviewer and CODEOWNER, I have reviewed this and have,才能触发签核验证。
  • **PR 验证:**扫描仅在带有标签的 PR 上运行。强烈建议添加 full-sweep-fail-fast;仅当需要矩阵任务在失败后继续运行时才使用 full-sweep-enabled。
  • **合并后:**PR 作者必须确保所有 GitHub Actions 任务通过。临时性失败通常可以通过重新运行恢复;参见重新运行失败任务的说明。

@github-actions

github-actions Bot commented Sep 29, 2026 •

Copy link
Copy Markdown
Contributor

@claude claude Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, straightforward config alignment change.
What was reviewed: verified each of the four bumped model.container values against the corresponding (unchanged) master-config image in amd-master.yaml/nvidia-master.yaml — all match exactly; checked that every cuda-graph-max-bs occurrence in the two touched v0.5.20 sglang recipes was renamed to cuda-graph-max-bs-decode, and confirmed no other recipe in the repo still pairs the v0.5.20 sglang image with the old flag name; confirmed the new test drives real recipe expansion (generate_override_configs via selected_recipes) rather than pinning static config, and that the perf-changelog diff only appends a new entry at the file's tail.

Extended reasoning...

The diff is confined to four recipe YAML image/flag edits, a new pure-Python static test, and an append-only changelog entry — no auth, crypto, or data-exposure surface. I cross-checked all four container values against the master configs directly and confirmed they match, verified the flag rename is complete and scoped correctly, and confirmed the new test exercises real config-selection logic rather than pinning literals; the perf-changelog append preserves history. None of the touched paths fall under a CODEOWNERS-restricted pattern in .github/CODEOWNERS. The pull/XXX placeholder in the changelog PR-link is a known pre-merge fill-in matching existing repo convention, not a functional defect.

This review covers commit 75c7220, which is no longer the latest commit on this pull request; later commits are not covered by it.

Restore the MI325X GLM-5.2 and MI300X MiniMax-M3 recipes and drop the static image test from this PR; they will follow separately. The changelog entry now selects only the B200 and MI355X keys.

Co-authored-by: Cursor <[email protected]>
@chunfangamd chunfangamd changed the title fix(srt): align four single-node recipe images with master configs and add a static check / fix(srt):对齐四个单节点配方镜像与主配置并新增静态检查 fix(srt): align B200 and MI355X single-node recipe images with master configs / fix(srt):对齐 B200 与 MI355X 单节点配方镜像与主配置 Sep 29, 2026
chunfangamd and others added 2 commits September 29, 2026 03:41
…sistency

Co-authored-by: Cursor <[email protected]>

# Conflicts:
#	inferencex-e2e/perf-changelog.yaml
Restore the static check with the MI325X GLM-5.2 and MI300X MiniMax-M3 keys exempt until their recipes are aligned, and run CI Tests when master configs or srt-slurm recipes change so YAML-only PRs are checked before any GPU job.

Co-authored-by: Cursor <[email protected]>
@chunfangamd chunfangamd changed the title fix(srt): align B200 and MI355X single-node recipe images with master configs / fix(srt):对齐 B200 与 MI355X 单节点配方镜像与主配置 fix(srt): align B200 and MI355X recipe images with master configs and add a static check / fix(srt):对齐 B200 与 MI355X 配方镜像与主配置并新增静态检查 Sep 29, 2026
@chunfangamd

Copy link
Copy Markdown
Collaborator Author

Compared with #3334's passing run (same lmsysorg/sglang:v0.5.20-cu130 image, legacy script path)

Point Total tok/s per GPU Output tok/s per GPU TTFT p90
c128 52,006 vs 52,519 (−1.0%) 450.0 vs 452.6 (−0.6%) 24.1 s vs 25.7 s
c160 46,895 vs 50,043 (−6.3%) 388.5 vs 419.6 (−7.4%) 66.6 s vs 58.1 s

@chunfangamd

Copy link
Copy Markdown
Collaborator Author

/stage-results 36521758562

@github-actions

github-actions Bot commented Oct 1, 2026 •

Copy link
Copy Markdown
Contributor

@chunfangamd staged run 36521758562: https://inferencemax-app-git-staging-semianalysisai.vercel.app/inference?i_dates=2026-09-29~r36521758562

This run remains available across future /stage-results requests. Staging the same run ID again updates its staged data. Staging workflow

@chunfangamd

Copy link
Copy Markdown
Collaborator Author

/reuse-sweep-run 36521758562

Review showed the pytest scan cannot gate run-sweep, compares image sets per recipe instead of each matrix point's selected variant, misses EVAL_CONFIG_FILE, and pins checked-in config against the AGENTS.md test rules. A separate PR replaces it with a validator that the sweep calls before dispatch.

Co-authored-by: Cursor <[email protected]>
@chunfangamd chunfangamd changed the title fix(srt): align B200 and MI355X recipe images with master configs and add a static check / fix(srt):对齐 B200 与 MI355X 配方镜像与主配置并新增静态检查 fix(srt): align B200 and MI355X recipe images with master configs / fix(srt):对齐 B200 与 MI355X 配方镜像与主配置 Oct 1, 2026

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Status: No status

Development

Successfully merging this pull request may close these issues.

1 participant